Skip to content

[SPARK-59375][SQL] Check the array size limit in flatten codegen - #58707

Open
SEPURI-SAI-KRISHNA wants to merge 1 commit into
apache:masterfrom
SEPURI-SAI-KRISHNA:SPARK-59375-flatten-codegen-limit
Open

[SPARK-59375][SQL] Check the array size limit in flatten codegen#58707
SEPURI-SAI-KRISHNA wants to merge 1 commit into
apache:masterfrom
SEPURI-SAI-KRISHNA:SPARK-59375-flatten-codegen-limit

Conversation

@SEPURI-SAI-KRISHNA

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Flatten.genCodeForNumberOfElements accumulates the result element count into a long and hands it to CodeGenerator.createArrayData without checking it against ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH. Interpreted evaluation does check. This PR adds the same check to the generated code so both paths raise the same error.

This is the flatten half of the pattern that SPARK-58631 (#57832) fixed for array_repeat. concat has the identical gap and is fixed in #58703 (SPARK-59374).

Why are the changes needed?

The two evaluation paths report different error conditions for the same input:

  • Interpreted: COLLECTION_SIZE_LIMIT_EXCEEDED.FUNCTION, from QueryExecutionErrors.arrayFunctionWithElementsExceedLimitError.
  • Codegen: no check, so execution reaches ArrayData.allocateArrayData, whose own bound check raises the internal _LEGACY_ERROR_TEMP_2176 through QueryExecutionErrors.cannotCreateArrayWithElementsExceedLimitError.

An internal error condition is not meant to be user facing, and which one a query gets should not depend on whether codegen was used.

Worth noting for reviewers that this is latent rather than something a user hits today. Unlike array_repeat, where the element count comes straight from an argument, flatten needs its inputs to actually hold that many elements, so an executor will normally run out of memory before the limit is reached. The existing test reaches it only by using a ConstantColumnVector, which reports its length without allocating.

Does this PR introduce any user-facing change?

Yes, in the sense that the error condition changes for an input that already fails. flatten with more than MAX_ROUNDED_ARRAY_LENGTH total elements now raises COLLECTION_SIZE_LIMIT_EXCEEDED.FUNCTION under codegen, where it previously raised the internal _LEGACY_ERROR_TEMP_2176. That matches what interpreted evaluation has always raised. The set of inputs that fail is unchanged.

Added a migration guide entry alongside the one #57832 added for array_repeat and array_insert, and the one #58703 adds for concat, since this is the same kind of change. Happy to drop it if you would rather not document a case that is this hard to reach.

How was this patch tested?

Extended the existing Elements exceed limit for flatten() test in QueryExecutionErrorsSuite to run the expression through GenerateUnsafeProjection as well as eval, asserting the same error from both. That test already builds a large array cheaply with ConstantColumnVector, which reports its length without allocating, so nothing large is allocated.

It sits in QueryExecutionErrorsSuite rather than in CollectionExpressionsSuite, where #57832 put its tests, because ConstantColumnVector lives in sql/core and is not reachable from a catalyst suite. Extending the existing test also seemed better than adding a second one: the test was already there and simply never exercised codegen, which is how the gap survived.

Reverting the change makes the new codegen case fail with _LEGACY_ERROR_TEMP_2176, which is the behaviour being fixed.

build/sbt "sql/testOnly org.apache.spark.sql.errors.QueryExecutionErrorsSuite -- -z \"Elements exceed limit\""

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 5)

Flatten.genCodeForNumberOfElements accumulated the result element count
without checking it against MAX_ROUNDED_ARRAY_LENGTH, while interpreted
evaluation did check. The two paths therefore reported different error
conditions for the same input: COLLECTION_SIZE_LIMIT_EXCEEDED.FUNCTION
from eval, and the internal _LEGACY_ERROR_TEMP_2176 from codegen, raised
by ArrayData.allocateArrayData once execution reached it.

Raise the same error from the generated code, extend the existing
limit test to cover the codegen path as well as interpreted evaluation,
and document the change in the migration guide alongside SPARK-58631.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant